Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @workingjubilee (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Pointers backing Vecs of ZSTs can be dangling without capacity being 0.

Huh. My understanding was always that the zero-length Vec should be obtained specifically from dangling().

I would like it if you could clarify the motivation here.

The argument is that reconstructing a Vec may be unsound if this is not made true, because of Vec::new()'s internals.

However, it isn't actually language UB. This is about the library's contracts, and you're trying to argue the library is breaking its contract with... itself. Which it is allowed to do, as long as this doesn't lead to language UB. And it doesn't, as far as I understand. And more specifically, it is allowed to define an unfair contract, such that e.g. operating on a pointer "allocated" via Vec::new() is acceptable but you calling dangling() is not.

So, what do you actually have to say to the response of "I am the standard library. I make the rules."?

@workingjubilee
this singular line right here:

These requirements are always upheld by any ptr that has been allocated via Vec<T, A>.

means that these "prerequisites" are not just that, but they are also a guarantee for Vec::as_raw_ptr and into_raw_parts, and the standard library does not uphold these guarantees as they are written today.

Hmm.

By simultaneously requiring that ptr is allocated by the global allocator and saying that Vec upholds the requirements, it's also saying that ptr being allocated by the global allocator is an invariant of Vec (at least as far as users are concerned).

This means that deallocating pointers obtained from Vec should be sound, but it's not. It's easy to make a custom global allocator that meets the requirements on GlobalAlloc but causes UB when trying to dealloc dangling pointers.

If the stdlib wants to prevent users from constructing dangling ptr manually, an alternative solution would be to require that ptr meets the existing requirements or is obtained from a Vec.

Edit: Note that I don't think the alternative solution I mentioned is a good one. I don't see a good motivation for making the use of manually created dangling pointers unsound. I also think that this would make the docs harder to read.

Ah. Noticed that since this PR was opened, this one was also: #129748

That makes it far less likely that Vec::from_raw_parts should (notionally) deviate.

because you shouldn't have to check to make sure std didn't pass you the dangling pointer or anything.

it would be impossible to do this check, since there's not just one dangling pointer in existence, and also because the return value of NonNull::dangling may coincidentally be a valid pointer.

Requiring libs-api signoff for this seems fair to me. For that I would like to bring up std::slice::from_raw_parts as relevant, which (from my reading) already accepts manually created dangling pointers and whose language is much clearer to me (though that is in part because the functionality is less complex).

slices aren't container types, they're pointers.

Even though slices aren't container types they are integrated with the Vec API through slice::into_vec.

You can use std::slice::from_raw_parts (well actually, using std::ptr::slice_from_raw_parts is better) together with Box::from_raw and slice::into_vec to implement your own Vec::from_raw_parts that accepts any smaller than isize::MAX, aligned and non-null pointer when capacity or size_of::<T>() are 0: playground.

    pub fn into_vec<A: Allocator>(self: Box<Self, A>) -> Vec<T, A> {
        // N.B., see the `hack` module in this file for more details.
        hack::into_vec(self)
    }

I love it when std's code has comments like this.

Hmm, I see. That does constrain things a lot. I don't think we have a realistic choice otherwise, then.

I will re-review this for conformance with changes like #129748 and those formerly tracked by the now-closed #117945

@lolbinarycat It might be appropriate to add comments in this file that capture the fact that our implementation choices are stapled down by things like slice::into_vec.

@oskgo Thank you for finding that example, it was basically what I was wondering if it already existed or not. It didn't occur to me to go find it on another type that isn't Vec.

@rustbot author

You can use std::slice::from_raw_parts (well actually, using std::ptr::slice_from_raw_parts is better) together with Box::from_raw and slice::into_vec to implement your own Vec::from_raw_parts that accepts any smaller than isize::MAX, aligned and non-null pointer when capacity or size_of::() are 0: playground.

We don't guarantee that the Vec you end up with has the same pointer as what you started with, do we?

Though at this point we're getting deep into rules lawyering territory and there is probably little justification for not also guaranteeing this on Vec::from_raw_parts.